test(core-components): promote duplicate-dom-ids-regression to @stable (#1109) - #1327
Merged
Victor-w-Madeira merged 4 commits intoAug 6, 2026
Merged
Conversation
#1109) The upstream fix this spec asserts (langflow#14312 / LE-2037) has reached the release line the nightly is cut from: get-node-scoped-dom-id.ts now resolves on release-1.12.0 at the same sha as release-1.11.2 and main (a8a903e). Both cases run green on langflowai/langflow-nightly:latest (1.12.0.dev18) and, per the issue's criterion, for the right reason -- the ids read popover-anchor-input-url_input-APIRequest-<suffix> and textarea_str_system_prompt-Agent-<suffix>, while the field data-testid still resolves to 2 elements. Tag, spec doc and checklist bullet only; no test logic changed.
Contributor
There was a problem hiding this comment.
🟢 Ready to approve
Only minor doc wording cleanup is suggested; the functional change is limited to adding @stable tags and aligning the checklist/doc accordingly.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR promotes the existing duplicate DOM-id regression coverage for node parameter fields to @stable, ensuring it runs in the weekday daily-stable.yml lane now that the upstream fix is present in the nightly release line.
Changes:
- Add
@stableto both tests incore-components/duplicate-dom-ids-regression.spec.ts. - Update the corresponding QA checklist bullet from
[-]to[x]. - Refresh the spec doc to reflect the promotion, updated validation baseline (1.12.x), and the release-line rationale.
File summaries
| File | Description |
|---|---|
| tests/tests-automations/regression/core-components/duplicate-dom-ids-regression.spec.ts | Adds @stable tags to both regression tests. |
| QA-CHECKLIST.md | Updates the manual Part II bullet to mark the coverage as validated ([x]). |
| docs/core-components/duplicate-dom-ids-regression.md | Updates tags/validation context and documents why/when @stable was added. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| **`@stable` is deliberately absent, and this is the reason.** The upstream fix landed on the **`release-1.11.2`** branch (PR merged 2026-07-29). `langflowai/langflow-nightly:latest` — the image `daily-stable.yml` runs against — is built from the highest `release-*` branch, currently `release-1.12.0`, where the helper `get-node-scoped-dom-id.ts` is **verifiably absent** (it 404s on both `main` and `release-1.12.0`). Both cases therefore hard-fail on today's nightly *by design*. Tagging them `@stable` would open a `daily-failure` issue every weekday and trigger the `auto-remove-stable` path, which would strip the tag and commit to `main` — burning triage cycles to rediscover something already known. | ||
|
|
||
| `@stable` should be added once the fix reaches `main` / the 1.12 line and a run against the nightly confirms both cases green. Verify with: | ||
| **`@stable` was deliberately withheld until the fix reached the 1.12 line, and it now has (#1109).** When this spec was merged (#1102 / PR #1106) the upstream fix existed only on the **`release-1.11.2`** branch, while `langflowai/langflow-nightly:latest` — the image `daily-stable.yml` runs — is built from the highest `release-*` branch. Both cases therefore hard-failed on the nightly *by design*, and tagging them `@stable` then would have opened a `daily-failure` issue every weekday and tripped `auto-remove-stable` into stripping the tag and committing to `main`. The helper now resolves on `main` **and** on `release-1.12.0` at the same sha (`a8a903e`), and both cases run green on `1.12.0.dev18`: |
…dit (#1109) From an independent review of the promotion. expectFieldIdsUniquePerNode read the field ids through evaluateAll, which resolves the selector once with no auto-wait. The only prior gate was .react-flow__node reaching 2, which proves the NODES mounted, not their parameter fields, so a field arriving one commit later read as `got 0`. That is a hard, well-named failure rather than a vacuous pass -- but on the daily a hard failure strips @stable automatically and commits to main, so the race is worth closing rather than triaging later. Force-failed at toHaveCount(3) on both cases. Doc repairs, all introduced by the promotion edit itself: - the Tags section opened on a truncated sentence ("and it now has"); - rewriting Preconditions removed the only bullet identifying which build the "build carrying langflow#14312" measurement row referred to, leaving that row unanchored and undatable; it is now named inline; - the row added for 1.12.0.dev18 recorded literal DOM ids whose suffix is a per-node nonce regenerated on every mount -- four runs produced four different pairs -- so it read as if a literal suffix were a contract; - Tags and Preconditions both asserted the no-credential claim and pointed at each other; Preconditions now carries the measurement (green with no provider configured locally, green after Collect models on the PR lane) and Tags the rationale; - the @agentS note understated the tooling: provider-dependent-specs.mjs reads the tag file-scoped as a provider declaration, which both forces the Collect models sweep on a PR that changes this spec and excludes it from a PR that only changes a helper it imports. Also records that the canvas sweep only widens coverage on Case B: a two-node API Request canvas carries exactly 2 form controls with an id, the same two the field assertion already covers, while the two-Agent canvas carries 4.
Last item of the independent review. Case A gates on the first node rendering before adding the second, with a comment explaining why the sidebar interaction is fire-and-forget; Case B fired both dragTo calls back to back and let the final toHaveCount(2) catch whatever went missing, 15 s later and without saying which of the two drags was lost. #1304 measured 4/20 swallowed sidebar interactions on this nightly line, so the path is live. The loop now asserts the running node count after each drop and names the drag in the failure message. Force-failed at index + 2: "Agent drag 1/2 did not land on the canvas -- Expected 2, Received 1".
Victor-w-Madeira
deleted the
test/issue-1109-promote-duplicate-dom-ids-stable
branch
August 6, 2026 12:47
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1109.
core-components/duplicate-dom-ids-regression.spec.tswas merged without@stabledeliberately (#1102 / PR #1106): the upstream fix it asserts — langflow#14312 (LE-2037 /
langflow-ai/langflow#14096) — existed only on the
release-1.11.2branch, whilelangflowai/langflow-nightly:latestis built from the highestrelease-*branch. Bothcases hard-failed on the nightly by design, and tagging them then would have opened a
daily-failureissue every weekday and trippedauto-remove-stableinto stripping the tagand committing to
main.With
nightly.ymldisabled, that left the spec running nowhere except a PR'simpacted-specs job — and not reliably even there:
@agentsmakesscripts/provider-dependent-specs.mjsclassify it provider-dependent, so a PR thatmerely changed a helper it imports would have excluded it. #1109 was the tracker for
the other half of that decision.
The trigger fired — on the ref that decides it
Not
main. The nightly is cut from the release line, so the check that matters isrelease-1.12.0, and the helper now resolves on both at the same sha:1. Promoted tests
two API Request nodes expose the same field without duplicating its DOM idpopover-anchor-input-url_inputresolves to exactly 2 elements (thedata-testidstays unscoped — the contract 132 call sites across 45 specs depend on), both carry a non-empty DOM id, the two ids differ, and the canvas-scopedinput/textarea/selectsweep returns[]two Agent nodes expose the same field without duplicating its DOM idtextarea_str_system_promptacross two dragged Agent nodes. The Agent is placed and inspected, never executed — no LLM call, no credential2. Green for the right reason, not merely green
The issue's acceptance criterion was that the ids read
…-APIRequest-<suffix>/…-Agent-<suffix>, not just that the assertions pass. Captured from the force-fail runs,which print the real values:
popover-anchor-input-input_valueon the Agent — a pre-fix duplicate recorded in the specdoc's measurement table — is scoped as well. The field testid still resolved to 2
elements in both cases, so the contract half stayed green across the fix.
3. Force-failability audit
A promotion is where a spec that cannot fail becomes a blind spot in the daily, so the file
was read for dead assertions,
if (visible)bodies and silent early-return chains beforetrusting the green. It has none — every assertion is unconditional and
collectDuplicateFormFieldIdsthrows when the canvas root is missing rather than returningan empty list. Nothing needed hardening; the mutations below prove all three assertions are
live, per test, isolated with
--grep.4. Scope, and the one logic change
The promotion itself is tag + doc + checklist bullet. An independent review of that commit
found two races, both fixed here. Neither can cause a wrong verdict — both fail hard
and are never a vacuous pass — but on the daily a hard failure strips
@stableautomatically and commits to
main, so they are worth closing before promotion rather thantriaging after it.
expectFieldIdsUniquePerNoderead the ids throughevaluateAll, which resolves the selector once, with no auto-wait. The only priorgate was
.react-flow__nodereaching 2 — that proves the nodes mounted, not theirparameter fields, so a field arriving one commit later read as
got 0. Now gated on thefield testid reaching 2 first. Force-failed at
toHaveCount(3)on both cases.before adding the second, and says why in its own comment; Case B fired both
dragTocalls back to back and let the final
toHaveCount(2)catch whatever went missing — 15 slater, without naming which drag was lost. [Daily #1296] modelInputComponent — the canvas renders no node at all, so the model-name trigger is never reachable (hard failure, no outage cover) #1304 measured 4/20 swallowed sidebar
interactions on this nightly line, so the path is live. The loop now asserts the running
count after each drop. Force-failed at
index + 2: "Agent drag 1/2 did not land on thecanvas — Expected 2, Received 1".
The same review found five defects introduced by the promotion's own doc edit — a
truncated opening sentence in Tags, a measurement row left unanchored when the bullet
naming its build was rewritten away, literal DOM ids recorded as if a per-node nonce were a
contract, a circular duplicate claim between Tags and Preconditions, and an
@agentsnote that understated the tooling. All repaired in the same commit.
5.
@agents— the tag is a subject, the tooling reads it as a dependencyNot changed here, but it has two live consequences and the PR should name both.
scripts/provider-dependent-specs.mjstreats@agentsas a provider declaration,file-scoped, so it marks both cases even though neither executes the Agent:
Collect modelssweep,which is a hard gate on
pr-validation.yml. Confirmed on this PR's own run:Collect modelsexecuted for 1.3 min with all five keys. So a tag-and-doc PR is coupled toprovider-key health; the day a key drains (OpenAI API key out of quota — OpenAI-bound @stable specs hard-fail every daily #772 / Provider-hardcoded specs gate on the env var, not on provider health — a drained key wedges the shard #1029 / pr-validation: the lane pays for multi-provider LLM coverage it does not owe (141 runs / 3.5 days on claude-sonnet-5) #1169) it goes red for a reason
unrelated to its diff.
Neither is a bug in that mechanism; both rest on a premise this file does not meet. The
spec doc now records it. Changing the classifier or the tag is a separate question and is
not smuggled into a promotion.
Validation (nightly
1.12.0.dev18,--retries=0 --workers=1)edit. After the first: 7.9 s / 6.7 s / 6.6 s. After the second: 7.3 s / 7.0 s / 6.9 s.
2 passed every time, 10 runs in total ✅
1.12.0.dev18afterCollect models— 2 passed (10.4 s) ✅--trace=on✅ — 8.4 s, no hang (this spec places nodes, it does not load the SimpleAgent template whose canvas hangs tracing)
🚨 Backend Erroron every run ✅new Set(ids).size→.toBe(3)→ 1 failed in both tests, printing the realnode-scoped ids
count > 1→count > 0→ 1 failed in both (Case A named bothurl_inputids; Case B all four)toHaveLength(2)→toHaveLength(3)→ 1 failed in bothtoHaveCount(2)→toHaveCount(3)→ 1 failed in bothtoHaveCount(index + 1)→toHaveCount(index + 2)→ 1 failed,naming the drag
grep FFMUT= 0, and the spec diff was empty before the tag edit ✅after 6 executions ✅
npm run typecheck✅ ·npm run lint0 errors ✅ · QA-CHECKLIST generated-block guard ✅ ·check:checklist-coverage✅ ·--grep @stablenow selects both tests ✅QA-CHECKLIST.md: the manual §2.1 bullet only ([-]→[x]).coverage:summarywas notrun and no generated block is committed (Guard QA-CHECKLIST generated blocks from PR edits (stop recurring merge conflicts) #741).